Skip to content

Add Hindcast Workflow - #24

Merged
mxkpp merged 12 commits into
developmentfrom
maxkipp-hindcast
Jul 22, 2026
Merged

Add Hindcast Workflow#24
mxkpp merged 12 commits into
developmentfrom
maxkipp-hindcast

Conversation

@mxkpp

@mxkpp mxkpp commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Adds a new way of calling nwm-fcst-mgr's run_hindcast function as a generator of RealizationBuilder instances (skipping ngen run) to allow nwm-rte itself to run ngen on each realization yielded, through the formal NgenRunnerAsync wrapper.

This requires: NGWPC/nwm-fcst-mgr#43

Additions

  • New CLI arg -hc, --hindcast-args which contains 2 parts:
    1. cycle_interval : int (required when -hc provided)
        Cycle interval (in hours) between hindcast runs.
    2. num_iterations : int (required when -hc provided)
        Number of hindcast cycles to perform.
  • New example comand in run_suite.sh:
docker_run python -um "ngen_rte.run_forecast" -fconfig "short_range" -dt "2025-07-10 04:00:00" -rname "fcst_run1_short_range_hindcast" -hc 3 10

Removals

Changes

  • run_forecast.py (RealizationBuilder.run_types "forecast", "cold_start", "hindcast", "warm_start") now write logs to rb.work_dir instead of to rb.input_dir, by default.
  • RealizationBuilder run types "hindcast" and "warm_start" now leverage the new "do_override_log_file_prefix" arg from nwm-fcst-mgr's ForecastExecutionManager.preprocess.

Testing

  1. Ran the new example command and watched the hindcast and warm_start realizations get built and ran.

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

@mxkpp
mxkpp requested a review from jswade-rtx July 21, 2026 20:19
jswade-rtx
jswade-rtx reviewed Jul 21, 2026
View reviewed changes
Comment thread bin_mounted/ngen_rte/configs.py Outdated
@@ -199,8 +214,8 @@ def configure_ngen_log(self, rb: RealizationBuilder) -> None:

if rb.run_type in ("default", "checkpoint", "regionalization"):

@jswade-rtx jswade-rtx Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that these encompass all run types, coud we just set fallback_log_dir = str(rb.workdir) without a check for run type?

@mxkpp mxkpp Jul 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I kept the current conditional flow in case there may be some need for the others to be separated later, but currently there is no need for them to be separated.

@jswade-rtx jswade-rtx Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say we just condense it to a single line.

@mxkpp mxkpp Jul 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

jswade-rtx
jswade-rtx reviewed Jul 21, 2026
View reviewed changes
Comment thread bin_mounted/ngen_rte/configs.py Outdated
f"Hindcast _num_iterations must be str representation of an integer, but got: {repr(_num_iterations)}"
)
)
self.hc_cycle_interval = int(_cycle_interval)

@jswade-rtx jswade-rtx Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an error appended when matching the cycle_interval in L297, the int(_cycle_interval) here will likely fail before those errors can be logged properly.

@mxkpp mxkpp Jul 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved error handling

jswade-rtx
jswade-rtx reviewed Jul 21, 2026
View reviewed changes
Comment thread bin_mounted/ngen_rte/configs.py Outdated
)
)

if self.hc_cold_start_state:

@jswade-rtx jswade-rtx Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already discussed with Max, but we should be able to hook up cold starts to hindcasts in the RTE.

@mxkpp mxkpp Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reading a stale note too literally in a ticket -- yes this should be easy to wire up to existing coldstart/warmstart support for starting a hindcast cycle.

@mxkpp mxkpp Jul 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented via cold_start_state=cfg.load_state_from

@mxkpp
mxkpp requested a review from jswade-rtx July 22, 2026 02:50
jswade-rtx
jswade-rtx approved these changes Jul 22, 2026
View reviewed changes

@jswade-rtx jswade-rtx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested state load for hindcasting and parallel processing for hindcasting. Looks good to me.

@mxkpp

mxkpp commented Jul 22, 2026

Copy link
Copy Markdown
Author

Marging after updating the example calls to include -n 2 (and confirming that it runs under that condition).

@mxkpp
mxkpp merged commit fbcc962 into development Jul 22, 2026
5 checks passed
@mxkpp
mxkpp deleted the maxkipp-hindcast branch July 22, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jswade-rtx jswade-rtx jswade-rtx approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mxkpp @jswade-rtx